Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Eclipse Temurin, not AdoptOpenJDK in action #90

Merged
merged 2 commits into from
Mar 28, 2022

Conversation

MarkEWaite
Copy link
Contributor

The 'adopt' distribution has moved to Eclipse Temurin.

It won't be updated at the AdoptOpenJDK location.

See jenkinsci/jenkins#6406

https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt

https://github.com/actions/setup-java#usage

@MarkEWaite MarkEWaite requested a review from a team as a code owner March 27, 2022 14:29
@@ -47,9 +47,9 @@ jobs:
with:
fetch-depth: 0
- name: Set up JDK 8
uses: actions/setup-java@v2
uses: actions/setup-java@v3.0.0
Copy link
Member

@jglick jglick Mar 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uses: actions/setup-java@v3.0.0
uses: actions/setup-java@v3

https://github.com/actions/setup-java/tags https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-using-versioned-actions Does not seem to be possible to just say “latest”; you need to select a major version, though in this case the last major version was of no interest to GHA SaaS users IIUC.

There is little point in specifying an exact version, since this workflow is not run on PRs so there is no opportunity to test changes before merging.

@timja WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, low risk and less dependabot noise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was perplexed that dependabot did not offer the upgrade from v2 to v3. I specified precise version in hopes that dependabot would then recommend upgrades. I'm OK with either.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in hopes that dependabot would then recommend upgrades

It will. The point is that it will then be offering PRs for v3.0.1, v3.0.2, etc., but the person receiving these PRs will just be blindly merging them anyway since the PRs will all be green (modulo flaky tests or CI outages), even if the update would in fact break the next release.

Copy link
Contributor Author

@MarkEWaite MarkEWaite Mar 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in hopes that dependabot would then recommend upgrades

It will. The point is that it will then be offering PRs for v3.0.1, v3.0.2, etc., but the person receiving these PRs will just be blindly merging them anyway since the PRs will all be green (modulo flaky tests or CI outages), even if the update would in fact break the next release.

It seems like I have two uncomfortable alternatives.

  1. Use the precise version number and be notified of each update without being able to confirm the update works
  2. Use the major version number, accept that the most recent minor version will be used automatically, and accept that dependabot will not detect or report a new major version number

I think that your comment and the comment from @timja indicate that you prefer the second option. Is your preference strong enough that I should submit pull requests to the other repositories to implement that change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dependabot will not detect or report a new major version number

Oh wait I did not catch this before. Dependabot does not offer to update v2 to v3? I know it will offer major version updates like #85 in general. If true, this seems like a DB bug—is it filed?

Copy link
Contributor Author

@MarkEWaite MarkEWaite Mar 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait I did not catch this before. Dependabot does not offer to update v2 to v3? I know it will offer major version updates like #85 in general. If true, this seems like a DB bug—is it filed?

Good suggestion that it should be filed as a bug report. actions/[email protected] was released Feb 24, yet none of the repositories that I was tracking have ever had a Dependabot pull request proposed to update from actions/setup-java@v2 to actions/setup-java@v3. I think that means it is a dependabot bug.

Looks like it has been reported as dependabot/dependabot-core#4834

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We got a ton of PRs updating actions/setup-java@v2 to actions/setup-java@v3 this morning, as in jenkinsci/pipeline-input-step-plugin#85, so I think the upstream bug has been fixed.

Co-authored-by: Jesse Glick <[email protected]>
@jglick jglick merged commit b65ca5f into jenkinsci:master Mar 28, 2022
@MarkEWaite MarkEWaite deleted the use-temurin-for-cd branch March 28, 2022 14:03
MarkEWaite added a commit to MarkEWaite/platformlabeler-plugin that referenced this pull request Apr 11, 2022
MarkEWaite added a commit to MarkEWaite/schedule-build-plugin that referenced this pull request Apr 11, 2022
MarkEWaite added a commit to MarkEWaite/testng-plugin-plugin that referenced this pull request Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants